-
Notifications
You must be signed in to change notification settings - Fork 9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: make input HTML IDs unique in authorization-popup.jsx #9390
Conversation
In reality this is for 1Password's feature where you can create custom fields named like input IDs and 1Password fill prefill that.
You can test this yourself by going to http://localhost:3200/ and putting https://spartacus-demo.eastus.cloudapp.azure.com:8443/occ/v2/api-docs in the bar at the top, then going to Authorize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for contributing
@sethidden you'd have to address the e2e tests before we can merge this. |
Sure, I'll take a look at them within the next few days. Thank you for taking a look
…On November 20, 2023 1:05:45 PM GMT+01:00, "Vladimír Gorej" ***@***.***> wrote:
@sethidden you'd have to address the e2e tests before we can merge this.
--
Reply to this email directly or view it on GitHub:
#9390 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look fine! Thanks
Done ;] |
The HTML
id=
params "client_id" and "client_secret" applied to inputs in the autorization modal can become duplicated. HTML IDs are supposed to be unique within the document. This breaks password manager (1Password at least) prefills, as it gets confused about which field is whichDescription
Before you write me off as some HTML spec purist, the duplicate IDs actually break password managers.
![image](https://private-user-images.githubusercontent.com/5359825/283541165-7fa4a5d3-e3d1-4fae-b9e4-57da497c2ac4.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzU0MTE2NS03ZmE0YTVkMy1lM2QxLTRmYWUtYjllNC01N2RhNDk3YzJhYzQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YzEzMzc3ZjIyMjk2MjhiYjQ1YzU2YmE1ZWM4M2RiYjA3YTIzMmFiMGNjZjZlZjdkZmEwNmQzMTA0ODRjMzY2YSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.rzuOKgGY38KpLInG-Qqkgj5ft8PV7VXO8u-Lp_9ZHik)
There's a feature in 1Password where you can create a custom field with the same name as an input's ID
Like this (custom_id, client_secret, oauth_username, oauth_password):
Before the PR
Now, you may try and happily go to the Swagger authorization-popup.jsx view and try to prefill your fancy new entry and hope it matches the values to inputs correctly, but you will be met with this:
![image](https://private-user-images.githubusercontent.com/5359825/283539501-75f7c9d4-7a9c-4a1d-8a6e-a63bafb53371.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzUzOTUwMS03NWY3YzlkNC03YTljLTRhMWQtOGE2ZS1hNjNiYWZiNTMzNzEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YmY4ODkxNWJlYjc2ZGE3Zjk3M2U1ZmUyYmIxNTFkM2U2YzZhMGJhMzU0NTQ4Mjg2OGY2ZDE5OWJiNDAyMGQ1MiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.vir0_0JbzJ0HiotNxYjzI-MoFkiPCAawoJvfEIyJdbU)
Unfortunately, as you can see above, 1Password prefilled the first 3 fields correctly, but failed to fill the client_secret field.
You can go here to inspect the Swagger html yourself: https://spartacus-demo.eastus.cloudapp.azure.com:8443/occ/v2/swagger-ui/index.html
First client_secret input: (the one we wanted to be autofilled by 1Passowrd but wasn't)
![image](https://private-user-images.githubusercontent.com/5359825/283538514-b8a66247-4c47-43f1-a2c8-fd98e279a5c6.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzUzODUxNC1iOGE2NjI0Ny00YzQ3LTQzZjEtYTJjOC1mZDk4ZTI3OWE1YzYucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ZTQ3MTQyYTE0ZGYxZTE1ODkzOWI1OTM2ZGUxOWI1Nzc3MmQxNWQ0ZTAwYjdmYzBhODJhNzYxNzI2ZTZjMjY2NCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.c-mONTMXOB-x6eJVuVwwyP9QUTS6HjM3zi2GewsP4Qk)
Second client_secret input: (the one that was wrongly autofilled by 1password)
![image](https://private-user-images.githubusercontent.com/5359825/283538636-c1a2b0e7-a659-4473-83f1-5386313c1ed4.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzUzODYzNi1jMWEyYjBlNy1hNjU5LTQ0NzMtODNmMS01Mzg2MzEzYzFlZDQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NmYxZTMyNGQwYzUyNjRjMmNjMTUxZWEzMzgxMzQzMWU4ZWJkZGMzMTQxOWJhNjU0MWMzNjc4YmEwYWUwYjk0MCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.PXKaYAUjVzdhvrXVFW4uJXoMOStRPlciCkdpLQ5brFg)
As you can see, the IDs are exactly the same, which is why 1Password gets confused.
After the PR
With my PR, the inputs between sections have the _{flow} suffix, so they're unique. I added them for both the client_id and client_secret, even though only client_secret was problematic:
With my PR, we can see that 1Password prefills as expected
![image](https://private-user-images.githubusercontent.com/5359825/283542144-95c40c9a-6e2b-4735-bd64-57509245bf1d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzU0MjE0NC05NWM0MGM5YS02ZTJiLTQ3MzUtYmQ2NC01NzUwOTI0NWJmMWQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NmQ5ZDQ1ODU1MjVjNjBlMWFlMzhlNDU4NjFhNDk5ZDg5ZjMzMzUwOTc1M2Y0NTk3ZmQwMjJjMDU1MzNiODM3OSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.SSNk5XkkR-4Zq9wCWhuraRLmTybLwWt9kJ3JokGZvHU)
![image](https://private-user-images.githubusercontent.com/5359825/283542423-b85c31a8-f3a4-4e5e-b447-a18afc643929.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzU0MjQyMy1iODVjMzFhOC1mM2E0LTRlNWUtYjQ0Ny1hMThhZmM2NDM5MjkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YjI2Mzg5ZTM4MTA1NzY1YmFhNmE0ODIwODQ3NjQ2ZGY1N2ZiMDAyMjRiYmU1NDgwY2Y2NDhiNzAxZGYxNDdlZCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.ST6jbfgcbK3cAriQ26DGmGYIjma2qzdIiTIDvLnzlOs)
First set (the one that previously didn't prefill but now prefills fine):
and its complimentary "client_id" field:
Second filed (doesn't prefill now):
![image](https://private-user-images.githubusercontent.com/5359825/283542244-e37a72e2-3f7d-4e14-ab00-f308f074575b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzU0MjI0NC1lMzdhNzJlMi0zZjdkLTRlMTQtYWIwMC1mMzA4ZjA3NDU3NWIucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NWZjZGY0NDVkMWM2Yjc2Y2YyOTUwYjgwNjNkNmNhNmM0NDEzMzk2YzRhNWRkNzgxODJhNGM2OWY4NGFiMTZkOSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.YJQ_TPhVP2fRgfOjJLSuMyHgzVbpVeNZ111xi0cyIRI)
Naturally because the IDs changed I needed to change the 1Password field names as well:
![image](https://private-user-images.githubusercontent.com/5359825/283542637-ec4d8a05-c2e9-4fc7-893e-81710310c76d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwMzAxNjIsIm5iZiI6MTczOTAyOTg2MiwicGF0aCI6Ii81MzU5ODI1LzI4MzU0MjYzNy1lYzRkOGEwNS1jMmU5LTRmYzctODkzZS04MTcxMDMxMGM3NmQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMTU1MTAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NDI1OGY4MDExODU0MDkwNGVlOTA2NGNkNTIyMWM5YWI4YmQzMzQ2ZjFiMWQ3ZjA3YTNhNWIwMTM1ZDBjMmJhNiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.OXm1K4b54-H0t0hCWhwQNAy4r9h2kK3QjUTEqAAybQ0)
Motivation and Context
To improve prefilling Authorization fields with password managers and improve HTML spec compliance.
How Has This Been Tested?
Included in Description section.
Screenshots (if appropriate):
Included in previous sections.
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests